Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Table of Contents component #180

Merged
merged 14 commits into from
Jul 31, 2020
Merged

Table of Contents component #180

merged 14 commits into from
Jul 31, 2020

Conversation

kylesuss
Copy link
Collaborator

A highly dynamic table of contents component to be used in the consumers. The challenges w/ this component were mostly related to state management given the following constraints:

  • The TOC can have a random combination of different types of children (I called them "menu", "link", and "bullet link")
  • The TOC should be able to have the state of all of its children controlled at once externally (a button to expand/collapse all)
  • Changing pate path activates menu items that were not previously open
  • Keyboard navigation opens menu items that were not previously open so you can tab through the entire menu

domyen
domyen requested changes Jul 30, 2020
View reviewed changes
Copy link
Member

@domyen domyen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The size of this component is pretty daunting. Can we break ToC apart into two components: one for the "List" and the other for the "ListItems"?

@kylesuss kylesuss requested a review from domyen July 30, 2020 21:56
@kylesuss
Copy link
Collaborator Author

kylesuss commented Jul 30, 2020 •
edited
Loading

@domyen sure thing. I think it was one of those things I thought belonged together at the beginning because you'll never use them individually, but agreed that it got too big. I ended up separating more in this way:

  1. State management
  2. List
  3. List items

There is 1 caveat to that which is 1 of the list items is a recursive component render of the list iteself, so it is not broken out further (it lives in TableOfContentsItems).

domyen
domyen requested changes Jul 30, 2020
View reviewed changes
src/components/TableOfContents.js Outdated Show resolved Hide resolved
@kylesuss kylesuss requested a review from domyen July 30, 2020 22:53
@domyen
Copy link
Member

domyen commented Jul 31, 2020

lgtm, thanks @kylesuss

@kylesuss kylesuss merged commit 18640ba into master Jul 31, 2020
@kylesuss kylesuss deleted the toc branch July 31, 2020 18:24
@kylesuss
Copy link
Collaborator Author

🚀 PR was released in v5.1.6 🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@domyen domyen domyen approved these changes

Assignees

@domyen domyen

Labels
released
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@kylesuss @domyen